From ef8855fdc3615296db3d98c1a589fafc29aac2e4 Mon Sep 17 00:00:00 2001
From: Alex Crichton <alex@alexcrichton.com>
Date: Mon, 27 Oct 2014 11:54:48 -0700
Subject: [PATCH] Don't encode precise in lockfile dep pointers

Each dependency itself already has the precise source listed, and we're
guaranteed that for each (source, name) pair that there is exactly one
dependency, so there is always a way to rebuild the precise dependency graph
after the fact by looking up the (source, name) pair in the hash map.

This should help with some of the readability concerns in #637 because the git
SHA that a source is locked to is now only mentioned once in a lockfile.

This commit does preserve, however, the mention of the version in each
dependency line which will likely never go away. This means that for
registry-based packages will still run into the same lockfile merge conflict
troubles, there will just be more readable versions than git hashes.

It should also be noted that this will alter all currently generated lockfiles
as any dependencies mentioned will lose the hashes mentioned afterwards. This
will likely cause somewhat of a transitionary pain as this version of cargo
propagates throughout.
---
 src/cargo/core/package_id.rs      | 10 +++++
 src/cargo/core/resolver/encode.rs | 74 ++++++++++++++++++-------------
 2 files changed, 54 insertions(+), 30 deletions(-)

diff --git a/src/cargo/core/package_id.rs b/src/cargo/core/package_id.rs
index f9ee93b40..fffb9be84 100644
--- a/src/cargo/core/package_id.rs
+++ b/src/cargo/core/package_id.rs
@@ -132,6 +132,16 @@ impl PackageId {
 
         Metadata { metadata: metadata, extra_filename: extra_filename }
     }
+
+    pub fn with_precise(&self, precise: Option<String>) -> PackageId {
+        PackageId {
+            inner: Arc::new(PackageIdInner {
+                name: self.inner.name.to_string(),
+                version: self.inner.version.clone(),
+                source_id: self.inner.source_id.with_precise(precise),
+            }),
+        }
+    }
 }
 
 impl Metadata {
diff --git a/src/cargo/core/resolver/encode.rs b/src/cargo/core/resolver/encode.rs
index ec684d872..152be313d 100644
--- a/src/cargo/core/resolver/encode.rs
+++ b/src/cargo/core/resolver/encode.rs
@@ -20,48 +20,62 @@ pub type Metadata = TreeMap<String, String>;
 impl EncodableResolve {
     pub fn to_resolve(&self, default: &SourceId) -> CargoResult<Resolve> {
         let mut g = Graph::new();
+        let mut tmp = HashMap::new();
+
+        let packages = Vec::new();
+        let packages = self.package.as_ref().unwrap_or(&packages);
+
+        {
+            let register_pkg = |pkg: &EncodableDependency| {
+                let pkgid = try!(pkg.to_package_id(default));
+                let precise = pkgid.get_source_id().get_precise()
+                                   .map(|s| s.to_string());
+                assert!(tmp.insert(pkgid.clone(), precise),
+                        "a package was referenced twice in the lockfile");
+                g.add(try!(pkg.to_package_id(default)), []);
+                Ok(())
+            };
+
+            try!(register_pkg(&self.root));
+            for pkg in packages.iter() {
+                try!(register_pkg(pkg));
+            }
+        }
 
-        try!(add_pkg_to_graph(&mut g, &self.root, default));
-
-        match self.package {
-            Some(ref packages) => {
-                for dep in packages.iter() {
-                    try!(add_pkg_to_graph(&mut g, dep, default));
+        {
+            let add_dependencies = |pkg: &EncodableDependency| {
+                let package_id = try!(pkg.to_package_id(default));
+
+                let deps = match pkg.dependencies {
+                    Some(ref deps) => deps,
+                    None => return Ok(()),
+                };
+                for edge in deps.iter() {
+                    let to_depend_on = try!(edge.to_package_id(default));
+                    let precise_pkgid =
+                        tmp.find(&to_depend_on)
+                           .map(|p| to_depend_on.with_precise(p.clone()))
+                           .unwrap_or(to_depend_on.clone());
+                    g.link(package_id.clone(), precise_pkgid);
                 }
+                Ok(())
+            };
+
+            try!(add_dependencies(&self.root));
+            for pkg in packages.iter() {
+                try!(add_dependencies(pkg));
             }
-            None => {}
         }
 
-        let root = self.root.to_package_id(default);
         Ok(Resolve {
             graph: g,
-            root: try!(root),
+            root: try!(self.root.to_package_id(default)),
             features: HashMap::new(),
             metadata: self.metadata.clone(),
         })
     }
 }
 
-fn add_pkg_to_graph(g: &mut Graph<PackageId>,
-                    dep: &EncodableDependency,
-                    default: &SourceId)
-                    -> CargoResult<()>
-{
-    let package_id = try!(dep.to_package_id(default));
-    g.add(package_id.clone(), []);
-
-    match dep.dependencies {
-        Some(ref deps) => {
-            for edge in deps.iter() {
-                g.link(package_id.clone(), try!(edge.to_package_id(default)));
-            }
-        },
-        _ => ()
-    };
-
-    Ok(())
-}
-
 #[deriving(Encodable, Decodable, Show, PartialOrd, Ord, PartialEq, Eq)]
 pub struct EncodableDependency {
     name: String,
@@ -178,7 +192,7 @@ fn encodable_package_id(id: &PackageId, root: &PackageId) -> EncodablePackageId
     let source = if id.get_source_id() == root.get_source_id() {
         None
     } else {
-        Some(id.get_source_id().clone())
+        Some(id.get_source_id().with_precise(None))
     };
     EncodablePackageId {
         name: id.get_name().to_string(),
-- 
2.30.2